Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jcouls29/dynamic #70

Closed
wants to merge 8 commits into from
Closed

Conversation

Jcouls29
Copy link

Due to a need for a more dynamic setup of workflow, a static method was added to AutomatonymousStateMachine to allow for the builder pattern. New interfaces were created and all current tests were copied and converted to the builder pattern to ensure everything was up to spec.

The general principle is that, by using the builder pattern, anyone can build a system (or framework) on top of this library where the need to build the workflow within the constructor isn't ideal. By allowing the states and events to live outside of the workflow, allows for more flexibility for these type of use cases, especially in a dynamic workflow situation where configurable workflows may be necessary.

Open to any thoughts on this approach.

Example Usage

State Waiting;
Event Start;
Event First;
Event Second;
Event Third;

AutomatonymousStateMachine<Instance>
    .Build(builder => builder
        .State("Waiting", out Waiting)
        .Event("Start", out Start)
        .Event("First", out First)
        .Event("Second", out Second)
        .Event("Third", out Third)
        .Initially()
            .When(Start, b => b.TransitionTo(Waiting))
        .During(Waiting)
            .When(First, b => b.Then(context => { context.Instance.CalledAfterAll = false; }))
            .When(Second, b => b.Then(context => { context.Instance.CalledAfterAll = false; }))
        .CompositeEvent(Third, b => b.CompositeStatus, First, Second)
        .During(Waiting)
            .When(Third, b => b
                .Then(context =>
                {
                    context.Instance.Called = true;
                    context.Instance.CalledAfterAll = true;
                })
                .Finalize()
            )

    );

…tern

Added SetSuperState to primary State interface to allow for setting the super state when defining the state through the builder pattern

Modified scope of protected methods in AutomatonymousStateMachine to `protected internal` to allow the internal builders to access the methods without having to re-create them

Added new methods for defining State and Event to AutomatnymousStateMachine required by the builder pattern

Added Modify method, BuilderStateMachine class, and static Build method to AutomatonymousStateMachine to allow for the builder pattern to be initiated through the primary state machine. A proposed approach for a simple way to make it dynamic.

Added new interfaces and internal classes for the builder pattern.

Copied 165 of the 170 current tests, categorized them as "Dynamic Modify" and altered all state machine instances to utilize the builder pattern instead to ensure specs are still maintained.
Copy link
Member

@phatboyg phatboyg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this is pretty awesome, really great work. A few minor comments and ultimately this will make a nice feature for building state machines outside of the ctor.

src/Automatonymous/AutomatonymousStateMachine.cs Outdated Show resolved Hide resolved
src/Automatonymous/AutomatonymousStateMachine.cs Outdated Show resolved Hide resolved
src/Automatonymous/AutomatonymousStateMachine.cs Outdated Show resolved Hide resolved
src/Automatonymous/States/StateMachineState.cs Outdated Show resolved Hide resolved
@phatboyg
Copy link
Member

Is your intention to make this usable with MassTransit as well, or just Automatonymous?

@Jcouls29
Copy link
Author

In all honesty, I'm new to Automatonymous itself and haven't really touched MassTransit. I wouldn't feel comfortable doing any work outside of this library until I understand it more. If you're saying that this change will require some changes over there, I can definitely help modify to the best of my ability there as well. I had no intention of using MassTransit or modifying it.

…r namespace to avoid cluttering the root namespace
Removed some unnecessary comments.
@phatboyg
Copy link
Member

No need to modify MassTransit, so long as Automatonymous remains fully compatible with MassTransit after adding the builder features.

@Jcouls29
Copy link
Author

@phatboyg Sounds good. I tried to make these changes "additive" in nature and not cause any issues with how folks use it now. Just let me know about creating the static ASM class. If you'd like me to add that and alter the tests to use it, I can. Otherwise, I have completed all the other changes requested.

@phatboyg
Copy link
Member

I'll pull in your changes tonight and look them over again. Thanks!

@phatboyg
Copy link
Member

phatboyg commented Dec 1, 2020

Still on my todo list...

@phatboyg
Copy link
Member

This has been merged, I changed Create to New since Create is pretty commonly used as an Event name.

@phatboyg phatboyg closed this Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants